Migrate tools.go to Go 1.24 tool directives; remove dead gosec config from .golangci.yml - #48830
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Thanks for starting this refactoring work on the gosec module! The addition of Here are a few things to finalize:
Once these are done, this will be a great housekeeping PR that eliminates version-pin duplication across 4 separate files.
|
…onfig Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #48830 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
There was a problem hiding this comment.
The migration to Go 1.24 tool directives is clean and correct. Removing the dead gosec config from .golangci.yml reduces noise. One minor nit: in the Makefile security-gosec target, the new go tool gosec line is missing the @ prefix (all surrounding lines use @), so the command will be echoed to stdout on make. Not blocking — approve as-is or fix at your discretion.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 11.5 AIC · ⌖ 5.84 AIC · ⊞ 5.1K
There was a problem hiding this comment.
Skills-Based Review
Applied /codebase-design — requesting one minor fix.
Finding: go tool gosec ... in Makefile line 230 is missing the @ prefix, so the command echoes to stdout during make security-gosec — inconsistent with all other lines in that target.
Positive highlights:
- Excellent consolidation: tool versions declared once in go.mod
- Clean deletion of tools.go blank-import shim (Go 1.24 native approach)
- ~120 lines of genuinely dead gosec config removed from .golangci.yml
- CI workflow and Makefile updates are consistent
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 28 AIC · ⌖ 4.86 AIC · ⊞ 6.8K
Comment /matt to run again
There was a problem hiding this comment.
Approve — clean mechanical migration
Verified go build ./... succeeds with the new tool directives; the removed .golangci.yml gosec block was genuinely dead (gosec is disabled in golangci-lint per line 12-14), so nothing is lost. No correctness, security, or maintainability issues found in the changed lines.
💡 Review notes
go.modtool directives correctly list all three tools previously tracked viatools.goblank imports; versions are pinned via existingrequireentries (actionlint v1.7.12, gosec v2.28.0, x/vuln v1.6.0), so behavior is unchanged.- Makefile
security-gosec/security-govulncheck/security-govulncheck-sarif/toolstargets consistently switched togo tool <name>, removing the manualGOPATH/bin:PATHjuggling — a real simplification. security-scan.ymlworkflow mirrors the Makefile change (drops the explicitgo install ...@v2.28.0step in favor ofgo tool gosec).- Removed
.golangci.ymlgosec settings/exclude-rules were inert (gosec commented out ofenable:list), confirmed nolinters: gosecreference remains active elsewhere in the file. - Verified with
go build ./...locally — succeeds under go1.26.5 toolchain.
No blocking issues; nothing to comment on individual lines.
🔎 Code quality review by PR Code Quality Reviewer · sonnet50 · 51.2 AIC · ⌖ 4.53 AIC · ⊞ 7.5K
Comment /review to run again
|
@copilot run pr-finisher skill |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
This PR migrates Go tool dependency tracking from a tools.go pattern to go.mod tool directives, and updates local/CI commands to run security tooling via go tool.
Changes:
- Remove
tools.goand declare tool dependencies ingo.modvia atoolblock. - Update
Makefileand GitHub Actions security workflow to rungosec/govulncheckusinggo tool. - Clean up
.golangci.ymlby removinggosec-specific config and exclusions.
Show a summary per file
| File | Description |
|---|---|
| tools.go | Removes legacy tools.go-based tool pinning/imports. |
| go.mod | Adds tool directives to declare tool dependencies. |
| Makefile | Switches security scans to go tool and updates the tools install help text. |
| .golangci.yml | Removes gosec config/exclusions now that it’s not run via golangci-lint. |
| .github/workflows/security-scan.yml | Runs gosec/govulncheck via go tool instead of installing/running them directly. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 6
- Review effort level: Low
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
gosec version was hardcoded in 4 separate places (
go.mod, two Makefile targets, and the security workflow), and.golangci.ymlcarried ~30 lines of inert gosec settings/exclusions that never executed because gosec is disabled in golangci-lint.Go 1.24
tooldirectivesReplaces the
tools.goblank-import shim with nativetooldirectives ingo.mod:go.modis now the single source of truth for tool versionsmake security-gosecusesgo tool gosec ...— nogo install @hardcoded-versionorGOPATH/bin:PATHjugglingmake security-govulncheck/security-govulncheck-sarifusego tool govulncheckmake toolsinstalls viago install <path>without@version(resolves fromgo.mod).github/workflows/security-scan.ymlreplacedgo install ...@v2.28.0 && gosecwithgo tool gosectools.godeletedDead gosec config removed from
.golangci.ymllinters-settings.gosecblock (settings for a linter that's explicitly disabled)exclude-rulesentries withlinters: [gosec](none ever fired)Run: https://github.com/github/gh-aw/actions/runs/30446544004